Skip to content

Conversation

krismarc
Copy link
Contributor

@krismarc krismarc commented Jul 14, 2025

Fields support. No idea if it fits into your design. However, if you agree, I could come up with some tests, readme update etc.

Fields param as specified in the API description. Navigable object would contain only data from includes returned by fields query params.

fields = {"space.organization": ["guid", "name"], "space": ["guid,name,relationships.organization"] }
service_instances = client.v3.service_instances.list(fields=fields)

for service_instance in service_instances:
    space = service_instance.space()
    org = space.organization()
    print("Org name:" + org["name"])
    break
./test.py
debug: https://api.appcloudtest.company.com/v3/service_instances?fields[space.organization]=guid%2Cname&fields[space]=guid%2Cname%2Crelationships.organization
debug: manager_name: spaces
debug: manager_name: organizations
debug: ENTITY: {'guid': '6ce6284f-3024-4d4c-8df6-b6856454bba9', 'name': 'MY_ORG'}
debug: ENTITY: {'guid': '308da49b-f710-4a9f-b267-131d4f81cc0d', 'name': 'MAIN', 'relationships': {'organization': {'data': {'guid': '6ce6284f-3024-4d4c-8df6-b6856454bba9'}}}}
Org name: MY_ORG

Fields support.
https://v3-apidocs.cloudfoundry.org/version/3.197.0/index.html#fields
As specified in the API description. Navigable object would contain only data from includes returned by fields query params.
@krismarc krismarc mentioned this pull request Jul 14, 2025
@krismarc krismarc changed the title Update entities.py FIelds support Jul 14, 2025
@krismarc krismarc changed the title FIelds support Fields support Jul 14, 2025
@krismarc
Copy link
Contributor Author

I believe if proper handling for KeyError would be implemented here:
https://github.com/cloudfoundry-community/cf-python-client/blob/49089eef965ceb95babe1053cdf5b503a0c5f656/cloudfoundry_client/v3/entities.py#L218C1-L219C37
We could fallback and re-initiate accessed object if attribute accessed is missing in object crafted from includes returned by fields. So as long as you are looking for guid and name from fields, it wouldn't trigger extra calls. I believe you would know better how to handle it here. Somehow I am yet lost in this project implementation 😄

@antechrestos
Copy link
Member

@krismarc I am ok to offer the possibility to declare ask the backend to return only necessary fields; however, when it comes to KeyError, I would prefer not implement a fallback which could turn in a great complexity on code side.... And maybe in a endless loop if the accesssed key would not exist. If the caller asks for specific fields and tries to access some others... well... That's his/her problem.

I would like some unit tests just to garantee there is no issue..now and in the future.

After re-reading the code I see thats navigation and links already has fallback on empty object so I don't think it will break on missing fields

@krismarc
Copy link
Contributor Author

krismarc commented Jul 29, 2025

@antechrestos, sorry, I've missed your response. By any chance, are you able to generate test payloads? If not, I will handle somehow. Does it also mean, that you agree with the way how to declare that ask to the backend?
eg.
fields = {"space.organization": ["guid", "name"], "space": ["guid,name,relationships.organization"] }
service_instances = client.v3.service_instances.list(fields=fields)

Regarding that fallback, this was also my impression and why I suggested that you know the code better than anyone else 😄

btw, I've also raised a question with controller maintainers why there are no includes for service instances -> cloudfoundry/cloud_controller_ng#4450, I'd love this one in combination with this project. As I've mentioned here. In combination with includes, reading CF would be really fast.

@krismarc
Copy link
Contributor Author

damn.. while writing test, I've realized, that GET on a single object doesn't populate relationship with org. you simply have to trust that org returned in "includes" is the actual org of given space. So depending on the endpoint type you would have to handle differently.

That differs to actual real includes which always have relationship present.

image image

This API is too much overthought😅

To support this, we would have to blindly trust and create additional objects out of includes (if GET, not a LIST operation)
or append payload dynamically so this client knows how to handle it. Both options sound to me like repairing some inconsistency.

Alternative approach would be to not care about this and user needs to be aware that child objects are populated only if relationship was intentionally requested and returned by the api.

@krismarc
Copy link
Contributor Author

@antechrestos I've added a note within readme and tests for service instances. Would that be sufficient?

@antechrestos
Copy link
Member

@krismarc everything seems well despite the fact thtwe use lint. You can check linting by running

poetry run flake8 --show-source --statistics

@krismarc
Copy link
Contributor Author

krismarc commented Aug 4, 2025

@krismarc everything seems well despite the fact thtwe use lint. You can check linting by running

poetry run flake8 --show-source --statistics

done

@antechrestos antechrestos merged commit 686acc0 into cloudfoundry-community:main Aug 7, 2025
5 checks passed
@antechrestos
Copy link
Member

@krismarc thank you for your contribution.
I will make a fresh release by the next days.
Cheers 🍻

@antechrestos
Copy link
Member

@krismarc 1.39.0 released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants